-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix the snackbar notices position #18683
Conversation
} | ||
|
||
// Pad the scroll box so content on the bottom can be scrolled up. | ||
padding-bottom: 50vh; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this part in necessary for the typewriter experience.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we should restore it then, but it's not clear how it's related since it's very detached from the typewriter experience. Maybe it should be added in JS or in the typewriter component.
Also it's a separate regression from this PR I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not strictly needed for the typewriter experience. It just makes it nicer. It's styling and it can be adjusted. The comment says what it is: extra space on the bottom of the page so it can be scrolled up more. I'm not sure what's a better place for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe still this file then by overriding the editor regions style, EditorRegions is meant to be something generic that will be used outside the editor (widgets, ... any page that has the same global layout than the editor) so I don't think it makes sense there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ellatrix would you min adding the fix to this PR or a separate one. I'm personally not sure what impact this have and to be honest, the type writer experience feels good even without it for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really see any difference with these rules added/removed, but I'm also not sure when this class is added. Querying the DOM in the post editor returns 0 nodes for me. Overall reading this, removal of these rules feel unrelated to the fix here. Is this blocking @ellatrix ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These rules don't do anything at the moment because that class don't exist in the DOM. i think the fix should be done separately as this is just dead code for now. And the fix is not to bring the whole style back but just the padding bottom if I understand properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @youknowriad I verified that this fixes the snackbar position. Maybe let's drop changes to edit-post-layout__scrollable-container
since I didn't see how this was related in testing. If it's not actually needed, lets split that out to another janitorial.
This PR fixes another small regression caused by #18044 where the snackbar notices were positioned at the top instead of the bottom left of the page.
Basically the
edit-post-layout__content
className has been removed. Fortunately, this is the only place it was used. I'm also removing some useless styles I found.I'm also adding a "Needs dev note" label to document this change in the next WordPress release.